Skip to content

Comments

[feature] SC-166737/improve app proxy security by restricting where token replacements can go#89

Open
HappyPaul55 wants to merge 2 commits intomainfrom
SC-166737/improve-app-proxy-security-by-restricting-where-token-replacements-can-go
Open

[feature] SC-166737/improve app proxy security by restricting where token replacements can go#89
HappyPaul55 wants to merge 2 commits intomainfrom
SC-166737/improve-app-proxy-security-by-restricting-where-token-replacements-can-go

Conversation

@HappyPaul55
Copy link
Contributor

This pull request updates the Salesforce proxy configuration in the manifest.json file to improve security and flexibility when connecting to Salesforce services. The main change is the replacement of the wildcard URL pattern with a placeholder, and the addition of a mechanism to inject sensitive credentials from settings into request bodies.

Proxy configuration enhancements:

  • Changed the Salesforce proxy URL pattern from a wildcard to use the __salesforce_instance_url__ placeholder for improved explicitness and control.
  • Added a settingsInjection section to automatically inject client_key, client_secret, and global_access_token from settings into the request body as client_id, client_secret, and refresh_token respectively, enhancing credential management and security.

@HappyPaul55 HappyPaul55 requested a review from a team as a code owner November 18, 2025 15:46
@HappyPaul55 HappyPaul55 requested review from Copilot and removed request for a team November 18, 2025 15:46
@github-actions
Copy link

github-actions bot commented Nov 18, 2025

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request attempts to enhance the security of the Salesforce proxy configuration by replacing a wildcard URL pattern with a specific placeholder and introducing automatic credential injection via settingsInjection. The goal is to restrict proxy requests to only the configured Salesforce instance rather than allowing any Salesforce subdomain. However, the implementation has critical bugs that will prevent the proxy from functioning correctly.

Key intended changes:

  • Replace wildcard (.*).salesforce.com pattern with __salesforce_instance_url__ placeholder for tighter security
  • Add settingsInjection to automatically inject credentials into request bodies
  • Improve credential management by centralizing injection in the manifest configuration

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +151 to +161
"settingsInjection": {
"client_key": {
"body": ["client_id"]
},
"client_secret": {
"body": ["client_secret"]
},
"global_access_token": {
"body": ["refresh_token"]
}
}
Copy link

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The settingsInjection configuration appears to inject credentials into the request body for all requests to Salesforce services. However, examining src/api/api.ts lines 350-362, the OAuth token refresh logic already manually constructs a URL-encoded body with these credentials using placeholder syntax:

body: `grant_type=refresh_token&client_id=__client_key__&client_secret=__client_secret__&refresh_token=__global_access_token.json("[refreshToken]")__`

This creates a conflict where:

  1. The refresh token endpoint expects application/x-www-form-urlencoded content (line 355)
  2. Most other API calls use application/json content (line 329)
  3. The settingsInjection will inject into the body as JSON fields for all requests

This means regular API calls will have unnecessary credentials injected into their JSON bodies, and the OAuth refresh endpoint may receive duplicate or incorrectly formatted credentials. Consider restricting settingsInjection to specific endpoints or removing it if the manual placeholder approach in the code is the intended mechanism.

Suggested change
"settingsInjection": {
"client_key": {
"body": ["client_id"]
},
"client_secret": {
"body": ["client_secret"]
},
"global_access_token": {
"body": ["refresh_token"]
}
}
// Removed settingsInjection to prevent credentials from being injected into all requests

Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant